Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add table caption #850

Merged
merged 1 commit into from
Nov 10, 2020
Merged

Add table caption #850

merged 1 commit into from
Nov 10, 2020

Conversation

thorvaldj
Copy link
Contributor

@thorvaldj thorvaldj commented Nov 6, 2020 •
edited
Loading

An attempt in solving issue #621. Also added a simple test file for Table, covering the properties related to "Caption".

@vnys
Copy link
Member

vnys commented Nov 9, 2020

Looks good so far 👍 But there’s one test failing, mainly because of formatting issues. Have you opened the repo from root in your editor? And have you installed all the dependencies using pnpm? If so, you can run pnpm run lint ./libraries/core-react from root to see what fails – and pnpm run lint ./libraries/core-react -- --fix to try to fix it automatically. That will take care of the formatting at least, the other issue will have to be solved manually.

@thorvaldj thorvaldj force-pushed the caption branch 3 times, most recently from 1b2907a to 88f0d6e Compare November 9, 2020 19:51
@thorvaldj thorvaldj marked this pull request as ready for review November 9, 2020 20:00
@thorvaldj thorvaldj changed the title WIP: Add table caption Add table caption Nov 9, 2020
@thorvaldj
Copy link
Contributor Author

Looks good so far 👍 But there’s one test failing, mainly because of formatting issues. Have you opened the repo from root in your editor? And have you installed all the dependencies using pnpm? If so, you can run pnpm run lint ./libraries/core-react from root to see what fails – and pnpm run lint ./libraries/core-react -- --fix to try to fix it automatically. That will take care of the formatting at least, the other issue will have to be solved manually.

Yes, linting was the issue 👍 Thanks for the tip :-)

vnys
vnys approved these changes Nov 10, 2020
View reviewed changes
Copy link
Member

@vnys vnys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 Thanks for the contribution 😊

@vnys vnys merged commit 0275e3b into equinor:develop Nov 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vnys vnys vnys approved these changes

@mimarz mimarz Awaiting requested review from mimarz

@pomfrida pomfrida Awaiting requested review from pomfrida

@wenche wenche Awaiting requested review from wenche

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@thorvaldj @vnys